Skip to content

Conversation

@jwren
Copy link
Member

@jwren jwren commented Jul 24, 2025

This API is marked to be removed from the API in the platform, see https://github.com/JetBrains/intellij-community/blob/master/platform/platform-api/src/com/intellij/openapi/ui/ComponentWithBrowseButton.java#L270

This changes the output from the verifier from 4 usages of scheduled for removal API to 2 usages of scheduled for removal API

@jwren jwren requested review from helin24 and pq July 24, 2025 22:33
Copy link
Collaborator

@pq pq left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool! Have you confirmed that this works? Previously we were listening to the button and now the listener is attached to the text field. It seems like this will be triggered for text entry or clicks (which I don't think we want) but not for clicks on the browse button (which we do).

@jwren
Copy link
Member Author

jwren commented Jul 24, 2025

Cool! Have you confirmed that this works? Previously we were listening to the button and now the listener is attached to the text field. It seems like this will be triggered for text entry or clicks (which I don't think we want) but not for clicks on the browse button (which we do).

Yes, I validated that it works as intended.

@jwren jwren requested a review from pq July 24, 2025 23:00
@pq
Copy link
Collaborator

pq commented Jul 24, 2025

Cool. Thanks!

Looking closer, the API is odd and as it happens addActionListener just delegates to the button:

https://github.com/JetBrains/intellij-community/blob/0316cc7e494d2d7497daebf15402883d705b1144/platform/platform-api/src/com/intellij/openapi/ui/ComponentWithBrowseButton.java#L192-L194

(Not sure why they didn't stick to addBrowseListener which seems more legible to me! 🤷 )

@jwren jwren merged commit 6435ad4 into flutter:main Jul 24, 2025
4 checks passed
jwren added a commit that referenced this pull request Jul 28, 2025
This should be enforceable after the following land:
- #25
- #28
- #26
- #30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants